-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch prepDetections to gsub. #39
Conversation
…hange by reference.
The issue in #37 stems from the R default of printing 0 decimal seconds. This is internally converted to a character before strsplit/gsub, and so and millisecond information is dropped while attempting to create the "frac" column. Temporarily changing the R options inside of the function seems to fix this and so should close that issue. |
Hi @mhpob, I have looked into it and it seems that the issue in #37 can be solved by changing this line in prepDetections() This should also cater for the cases when fractional seconds are absent and be adequately fast. I like the Thanks, |
Great catch @baktoft -- that halved the time again and solved the warning on my data.
Columns
Is that something you would want to keep, or should I change things to strip that information? I had stripped it in 464e3f2 -- it does take a little longer there since that method has to convert to list time, then back to calendar time. |
Hmmm - it should be ok to truncate the fractional seconds from those columns. Something like this should be ok'ish in terms of cpu-time. |
Original result and time taken:
Current version using
Current version when
|
I'm now noticing that there are some floating point rounding issues (see rows 2 and 5 in the original result, e.g.). This was also the behavior in the original version of In your experience, does a thousandth of a second matter... especially since the clocks can drift on a much larger scale than that? |
Sorry - got side-tracked, but are now doing a bit of end-of-year-cleaning. 1/1000 of seconds can definitely matter in estimating positions (1/1000 ~ 1.5 meters), but the temporal resolution of systems yielding data to use with this function is only 1/1000 s, so the rounding will not be an issues here. |
* Switch to gsub. * Change options() to keep millisecond information. Copy raw data and change by reference. * Change frac calculation and drop resulting excess. * Explicity provide date format. Round off ts, epo, and frac. Co-authored-by: Mike O'Brien <[email protected]>
Allows speed increase outlined in #38.
Note: will give the error
when a match doesn't occurr. This happened for me on a few lines where fractional seconds weren't reported for some reason.